-
-
Notifications
You must be signed in to change notification settings - Fork 741
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[cmd] Add support for flags specifying filters type in vmmap, and allow multiple filters #1120
Conversation
I will write docs and tests later :P |
No problem, I'll make this a draft then to see the diff with that's ready to review. Change the status when ready |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting addition but I think you should rewrite the parsing loop using the parse_arguments
decorator here. It would gain in simplicity and clarity.
…_arguments` decorator (#1122) Add support for the `append` action for ArgumentParser in the `parse_arguments` decorator on optional arguments. This makes us able to support this: ``` command --arg 1 --arg 2 --arg 3 ``` And get `[1, 2, 3]` as the `arg` value. This CL is required to make changes to #1120 before merging. --------- Co-authored-by: crazy hugsy <[email protected]>
48ada57
to
ec99843
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly ok, just needs tests
93f588e
to
3da17fe
Compare
Co-authored-by: Grazfather <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm rn
We did it! |
Parse args:
-a
/--addr
:-n
/--name
: